From: Ian Campbell <ian.campbell@citrix.com>
Date: Mon, 2 Dec 2013 11:11:40 +0000 (+0000)
Subject: xen: arm: avoid truncation in mfn to paddr conversions
X-Git-Tag: archive/raspbian/4.8.0-1+rpi1~1^2~5824
X-Git-Url: https://dgit.raspbian.org/%22http:/www.example.com/cgi/%22https:/%22bookmarks://%22Dat/%22http:/www.example.com/cgi/%22https:/%22bookmarks:/%22Dat?a=commitdiff_plain;h=9ca423b1fb164a30f40bfacc6c03106fbe1945b1;p=xen.git

xen: arm: avoid truncation in mfn to paddr conversions

Although MFNs are 64-bit in the hypercall ABI they are most often unsigned
long internally, and therefore be 32-bit on arm32. Physical addresses are
always 64-bit via paddr_t.

This means that the common "mfn << PAGE_SHIFT" pattern risks losing some of
the top bits of the address is high enough. This need not imply a high amount
of RAM, just a sparse physical address map.

The correct form is ((paddr_t)mfn)<<PAGE_SHIFT and we have the pfn_to_paddr
macro which implements this. Grep for PAGE_SHIFT and << and switch to the
macro everywhere we can in the arch specific code. Note that page.h is
included by mm.h which defines the macro and so remains with the open coded
cast. I have inspected the common code matching this pattern and it uses the
correct casts where necessary (x86 also has pfn_to_paddr, so as a further
cleanup we could fix the common code too, but I haven't done that here).

I observed this as failure to boot a guest on midway, due to trying to map a
foreign page which belonged to no guest. I think this likely explains the
crashes which Julien has seen too.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 99e785aaaa..7c855cbc7d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -84,8 +84,8 @@ static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
         panic("Failed to allocate contiguous memory for dom0\n");
 
     spfn = page_to_mfn(pg);
-    start = spfn << PAGE_SHIFT;
-    size = (1 << order) << PAGE_SHIFT;
+    start = pfn_to_paddr(spfn);
+    size = pfn_to_paddr((1 << order));
 
     // 1:1 mapping
     printk("Populate P2M %#"PRIx64"->%#"PRIx64" (1:1 mapping for dom0)\n",
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 2de7dc7d0f..8189915132 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1032,10 +1032,10 @@ static int xenmem_add_to_physmap_one(
             return rc;
         }
 
-        maddr = p2m_lookup(od, idx << PAGE_SHIFT);
+        maddr = p2m_lookup(od, pfn_to_paddr(idx));
         if ( maddr == INVALID_PADDR )
         {
-            dump_p2m_lookup(od, idx << PAGE_SHIFT);
+            dump_p2m_lookup(od, pfn_to_paddr(idx));
             rcu_unlock_domain(od);
             return -EINVAL;
         }
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index af32511f56..1d5c841014 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -296,18 +296,20 @@ int guest_physmap_add_page(struct domain *d,
                            unsigned long mfn,
                            unsigned int page_order)
 {
-    return create_p2m_entries(d, INSERT, gpfn << PAGE_SHIFT,
-                              (gpfn + (1<<page_order)) << PAGE_SHIFT,
-                              mfn << PAGE_SHIFT, MATTR_MEM);
+    return create_p2m_entries(d, INSERT,
+                              pfn_to_paddr(gpfn),
+                              pfn_to_paddr(gpfn + (1<<page_order)),
+                              pfn_to_paddr(mfn), MATTR_MEM);
 }
 
 void guest_physmap_remove_page(struct domain *d,
                                unsigned long gpfn,
                                unsigned long mfn, unsigned int page_order)
 {
-    create_p2m_entries(d, REMOVE, gpfn << PAGE_SHIFT,
-                       (gpfn + (1<<page_order)) << PAGE_SHIFT,
-                       mfn << PAGE_SHIFT, MATTR_MEM);
+    create_p2m_entries(d, REMOVE,
+                       pfn_to_paddr(gpfn),
+                       pfn_to_paddr(gpfn + (1<<page_order)),
+                       pfn_to_paddr(mfn), MATTR_MEM);
 }
 
 int p2m_alloc_table(struct domain *d)
@@ -450,7 +452,7 @@ err:
 
 unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)
 {
-    paddr_t p = p2m_lookup(d, gpfn << PAGE_SHIFT);
+    paddr_t p = p2m_lookup(d, pfn_to_paddr(gpfn));
     return p >> PAGE_SHIFT;
 }
 
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index d252131e1a..0795eb9a64 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -409,7 +409,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
 
     do
     {
-        e = consider_modules(ram_start, ram_end, xenheap_pages<<PAGE_SHIFT,
+        e = consider_modules(ram_start, ram_end,
+                             pfn_to_paddr(xenheap_pages),
                              32<<20, 0);
         if ( e )
             break;
@@ -423,7 +424,7 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
     domheap_pages = heap_pages - xenheap_pages;
 
     early_printk("Xen heap: %"PRIpaddr"-%"PRIpaddr" (%lu pages)\n",
-                 e - (xenheap_pages << PAGE_SHIFT), e,
+                 e - (pfn_to_paddr(xenheap_pages)), e,
                  xenheap_pages);
     early_printk("Dom heap: %lu pages\n", domheap_pages);
 
@@ -471,8 +472,8 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
             e = ram_end;
 
         /* Avoid the xenheap */
-        if ( s < ((xenheap_mfn_start+xenheap_pages) << PAGE_SHIFT)
-             && (xenheap_mfn_start << PAGE_SHIFT) < e )
+        if ( s < pfn_to_paddr(xenheap_mfn_start+xenheap_pages)
+             && pfn_to_paddr(xenheap_mfn_start) < e )
         {
             e = pfn_to_paddr(xenheap_mfn_start);
             n = pfn_to_paddr(xenheap_mfn_start+xenheap_pages);
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index ce66099d26..b8d4e7d4fa 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -123,8 +123,8 @@ extern unsigned long xenheap_virt_end;
 #endif
 
 #define is_xen_fixed_mfn(mfn)                                   \
-    ((((mfn) << PAGE_SHIFT) >= virt_to_maddr(&_start)) &&       \
-     (((mfn) << PAGE_SHIFT) <= virt_to_maddr(&_end)))
+    ((pfn_to_paddr(mfn) >= virt_to_maddr(&_start)) &&       \
+     (pfn_to_paddr(mfn) <= virt_to_maddr(&_end)))
 
 #define page_get_owner(_p)    (_p)->v.inuse.domain
 #define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d))